-
Notifications
You must be signed in to change notification settings - Fork 8k
[RFC] Add clamp function #19434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[RFC] Add clamp function #19434
Conversation
a6d8707 to
d6f035f
Compare
|
The RFC page status is set to Withdrawn, update it to Draft if you have access. |
a499c4c to
408f386
Compare
f61c646 to
47446f8
Compare
47446f8 to
37cbb4a
Compare
|
Hello @kocsismate and @bukka, any news about this proposal to reconsider |
|
@kylekatarnls RFCs are discussed on the mailing list and then need to be voted on. See https://wiki.php.net/rfc/howto for an explanation of the process. |
|
Thanks. I'd like to participate there, but I failed to create a wiki account, the captcha stuff is asking me "To which email address do you have to send an email to now?" and re-reading the whole page, I tried |
That is accurate: What error message did you receive? |
|
Thanks for pointing me to the source code of the wiki, my error was |
f973850 to
b39d140
Compare
3450a45 to
386cbd4
Compare
386cbd4 to
240218d
Compare
Co-authored-by: thinkverse <hallberg.kim@gmail.com>
4606ae5 to
77b8827
Compare
ext/standard/math.c
Outdated
| Z_FLF_PARAM_ZVAL(2, zmin); | ||
| Z_FLF_PARAM_ZVAL(3, zmax); | ||
|
|
||
| if (EXPECTED(Z_TYPE_P(zmin) == IS_DOUBLE) && UNEXPECTED(zend_isnan(Z_DVAL_P(zmin)))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current code duplicates the code into the two different implementations.
It would be great to factor this out to a separate function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it all good? I used define assuming it's the most efficient way to share code without affecting runtime performances. Also is the position (just before the actual function) correct regarding conventions for the repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used
defineassuming it's the most efficient way to share code without affecting runtime performances.
No define, please. Just use a static function for the shared logic and the compiler will figure out whether or not inlining is reasonable (or whether it can just be a tail call or whatever).
Also is the position (just before the actual function) correct regarding conventions for the repository?
Position is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fully agree to use a proper function instead of defines.
You can pass the implicitly-defined zval *return_value variable to your helper.
e1bcf76 to
4f07c5d
Compare
Follow-up #7191
Implementation for the RFC: https://wiki.php.net/rfc/clamp_v2 (see also https://wiki.php.net/rfc/clamp)
Documentation: php/doc-en#4814